sql: fix FETCH FIRST on empty WITH HOLD cursor#171337
Conversation
FETCH FIRST on an empty persisted WITH HOLD cursor discarded the "no more rows" signal from sqlCursor.Next and returned true, causing the caller to decode an unset EncDatum. Propagate Next's (more, err) directly, matching the FetchAbsolute path. Fixes cockroachdb#171238 Release note (bug fix): FETCH FIRST on an empty WITH HOLD cursor no longer returns an internal error.
|
Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR. My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI. I have added a few people who may be able to assist in reviewing:
🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
An error occurred while handling your Trunk command: |
|
@yuzefovich friendly bump. This has been open about a week with CI green. Whenever you have a moment, would appreciate a look. Happy to address any feedback. |
DrewKimball
left a comment
There was a problem hiding this comment.
Nice work, thanks for the fix!
@DrewKimball reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on virajchogle).
|
/trunk merge |
Fixes #171238
FETCH FIRSTon a persistedWITH HOLDcursor over an empty result set returned an internal error (decoding unset EncDatum) instead of zero rows. InfetchMoveNodeBase.nextInternal, theFetchFirstbranch forcurRow == 0discarded the bool returned bysqlCursor.Nextand unconditionally reported a row, so the caller tried to decode an unset EncDatum.This propagates
Next's(more, err)directly from thecase 0branch, matching theFetchAbsolutepath just below.Related to #145362 / #145391 (prior fix for
FETCH ABSOLUTE), but the asymmetricFetchFirstpath was not reached by that change.Epic: none
Release note (bug fix): FETCH FIRST on an empty WITH HOLD cursor no longer returns an internal error.